Skip to content

fix: verify buyer ownership before entering the setinvoice flow#833

Merged
grunch merged 1 commit into
mainfrom
fix/setinvoice-buyer-ownership
Jun 12, 2026
Merged

fix: verify buyer ownership before entering the setinvoice flow#833
grunch merged 1 commit into
mainfrom
fix/setinvoice-buyer-ownership

Conversation

@grunch

@grunch grunch commented Jun 12, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Member

What

The addInvoicePHI handler (bot/commands.ts) and the setinvoice_ action (bot/start.ts) looked up the order using the id carried in the inline-button callback data and then entered the invoice flow, without checking that the caller is the order's buyer. This adds an explicit ownership check in both entry points so only the buyer can set or replace the payout invoice of their order.

Why

The payout invoice is the destination of the order's funds, so only the buyer should be able to provide or change it. Both entry points resolved the order purely from the callback id and trusted it, while the rest of the buyer-facing commands are scoped to the acting user. This brings these two paths in line with that model.

Change

  • bot/commands.ts — in addInvoicePHI, return early unless ctx.user is the order's buyer_id.
  • bot/start.ts — in the setinvoice_ action, return early unless ctx.user is the order's buyer_id.

Testing

  • npx tsc --noEmit passes.
  • npx eslint bot/commands.ts bot/start.ts passes.

Summary by CodeRabbit

  • Bug Fixes
    • Enhanced security for invoice payment operations by implementing buyer authorization verification. Only the order's buyer can now initiate or modify payout invoice details, preventing unauthorized access to sensitive payment information and ensuring proper transaction safeguards.

@grunch @claude
fix: verify buyer ownership before entering the setinvoice flow
e56aaf6 
The addInvoicePHI handler and the setinvoice_ action looked up the order
by the id carried in the callback data and then entered the invoice flow
without checking that the caller is the order's buyer. Add an explicit
check that ctx.user is the buyer in both entry points, so only the buyer
can set or replace the payout invoice of their order.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 12, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 75f3a02e-22c6-4da2-8990-cdaaf327c055

📥 Commits

Reviewing files that changed from the base of the PR and between 084c0da and e56aaf6.

📒 Files selected for processing (2)
  • bot/commands.ts
  • bot/start.ts

Walkthrough

Authorization checks are added to two invoice-related entry points: the addInvoicePHI function in bot/commands.ts and the setinvoice callback handler in bot/start.ts. Both verify that the request caller's user ID matches the order's buyer ID before proceeding, preventing unauthorized users from modifying payout invoices.

Changes

Invoice Buyer Authorization

Layer / File(s) Summary
Invoice buyer authorization guards
bot/commands.ts, bot/start.ts		 addInvoicePHI and the setinvoice callback handler each add an early-return check: the caller's ctx.user._id must match the order's buyer_id, or the request is rejected before invoice logic executes.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

  • lnp2pBot/bot#677: Both PRs modify the same invoice-related flow (addInvoicePHI and the /setinvoice handler), adding early-return preconditions (buyer-ownership in the main PR vs order-status/eligibility in #677).
  • lnp2pBot/bot#780: Both PRs modify the /setinvoice_<orderId> handler in bot/start.ts, adjusting control flow around which orders can advance into invoice-setting logic.

Suggested reviewers

  • Luquitasjeffrey
  • Catrya

Poem

🐰 A buyer alone may tweak their invoice with care,
No strangers shall slip through the payout affair!
Two guards now keep watch at each gate and each door,
Protecting each order's authorization core. ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: adding buyer ownership verification before entering the setinvoice flow, which is the core security fix across both modified files.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/setinvoice-buyer-ownership

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install failed due to a network error.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@grunch grunch requested a review from Luquitasjeffrey June 12, 2026 13:07
Luquitasjeffrey
Luquitasjeffrey approved these changes Jun 12, 2026
View reviewed changes

@Luquitasjeffrey Luquitasjeffrey left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tACK

@grunch grunch merged commit f05189c into main Jun 12, 2026
7 checks passed
@grunch grunch deleted the fix/setinvoice-buyer-ownership branch June 12, 2026 17:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Luquitasjeffrey Luquitasjeffrey Luquitasjeffrey approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@grunch @Luquitasjeffrey